Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

List recent pages and members on landing page #812

Merged
merged 14 commits into from
Aug 2, 2023
Merged

List recent pages and members on landing page #812

merged 14 commits into from
Aug 2, 2023

Conversation

mejo-
Copy link
Member

@mejo- mejo- commented Jul 31, 2023

📝 Summary

  • Show 10 most recent pages on landing page.
  • Show 3 last active members on landing page if it's not a public share.
  • Resolves: Improved landing page #311

🖼️ Screenshots

Overview
grafik
Screencast (slightly outdated)
recording1.webm

🚧 TODO

  • Animate scroll to left/right in recent pages slider
  • Update scroll buttons on scrolling (without using the buttons)
  • Don't display scroll to right button initially when not necessary
  • Show status in members avatars
  • Cypress tests

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

@cypress
Copy link

cypress bot commented Jul 31, 2023

Passing run #997 ↗︎

0 79 0 0 Flakiness 0

Details:

List recent pages and members on landing page
Project: Collectives Commit: 331a903c24
Status: Passed Duration: 05:21 💡
Started: Aug 2, 2023 3:14 PM Ended: Aug 2, 2023 3:20 PM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice! Would only say 2 things:

Show 3 last active members on landing page if it's not a public share.

Why not all until ellipsis is needed? It is nice to see faces. :)

And also a bit more vertical whitespace between the sections would be cool.

Copy link
Collaborator

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had a brief look at the code. So far everything seems fine. Just have one question.

src/components/Member/CurrentMembers.vue Show resolved Hide resolved
@juliushaertl
Copy link
Member

Looks really great already 😍

Just a minor UI nitpick:

  • The contrast of icons for pages without emojis seems a bit too low
  • I think the relative timestamp might need more space especially for languages that are a bit more verbose or if the name is longer

Screenshot 2023-08-01 at 16 44 44

Disable searching and actions for non-admin users.

Signed-off-by: Jonas <[email protected]>
Show list of members on landing page if it's not a public share.

Signed-off-by: Jonas <[email protected]>
* Make sure slider buttons don't interfere with scroll container and
  only get displayed when desired. Also fix background in active state.
* Use animated scrolling,
* Use scroll snap API to snap scrolling to beginning of tiles.
* Debounce updating of buttons.

Signed-off-by: Jonas <[email protected]>
@mejo-
Copy link
Member Author

mejo- commented Aug 2, 2023

I think the relative timestamp might need more space especially for languages that are a bit more verbose or if the name is longer

Unfortunately I don't have a good idea here. I'd rather avoid to make the tiles even wider, which would result in less tiles being displayed on narrow screens. And I configured it to be ellipsised on purpose to prevent randomly growing tile width. One option would be to add a line break, like "admin\nseconds ago". What do designers think @jancborchardt @nimishavijay?

@max-nextcloud
Copy link
Collaborator

Ideas that come to my mind:

  • as you said... use two rows: one for the user bubble one for the timestamp. Maybe even move the page title into the tile above?
  • Use the avatar only instead of the user bubble. Maybe with a tooltip with the user display name.

@jancborchardt
Copy link
Member

@max-nextcloud yep, avatar only seems fine. :)

@nimishavijay
Copy link
Member

@mejo- @max-nextcloud I would actually vote for the line break, for the following reasons:

  • since the avatar is so small it will most likely not be understood easily
  • we generally don't use the avatar in such a small size in other places
  • even if there is hover feedback, the touch target is only around 16-20px so possibly not accessible either. Especially on mobile I can imagine trying to click on the avatar and accidentally opening the page.

We could instead show the full user bubble on one line and the timestamp on the next line. That way the entire item could be clickable without any issues :)

@mejo-
Copy link
Member Author

mejo- commented Aug 2, 2023

grafik

vs.

grafik

Which one do you prefer @nimishavijay @jancborchardt @max-nextcloud 😊

I prefer the avatar only version. With the linebreak it doesn't work well in my eyes.

Our landing page doesn't have a title, so let's just reference it as
"Landing page".

Signed-off-by: Jonas <[email protected]>
@mejo-
Copy link
Member Author

mejo- commented Aug 2, 2023

I think I addressed all comments and suggestions for improvements now. See the commit history for details. The screenshot in the PR description is updated, the screencast is not.

@mejo- mejo- marked this pull request as ready for review August 2, 2023 13:13
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The headings (main page heading and also "Recent pages", "Collective members" and "Landing page") being grey instead of color-main-text looks a bit off, otherwise all good. :)

* Handle missing pageslider element gracefully.
* Calculate `scrollLeftMax` since it's non-standard and only supported
  on Firefox for now.
* Hide scrollbar on Chrome and Edge.

Signed-off-by: Jonas <[email protected]>
@mejo- mejo- merged commit 93630cd into main Aug 2, 2023
38 checks passed
@delete-merged-branch delete-merged-branch bot deleted the enh/landing_page branch August 2, 2023 15:23
@szaimen
Copy link
Contributor

szaimen commented Aug 8, 2023

🎉🎉🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved landing page
6 participants